-
Notifications
You must be signed in to change notification settings - Fork 266
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Histogram Zoom Preview on Study View #4957
base: master
Are you sure you want to change the base?
Add Histogram Zoom Preview on Study View #4957
Conversation
✅ Deploy Preview for cbioportalfrontend ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
821508f
to
7b990ad
Compare
<FlexAlignedCheckbox | ||
checked={ | ||
!!( | ||
this.props.chartControls && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simplify to !!this.props.chartControls?.showPreview
if (this._customDataBinFilterSet.has(uniqueKey)) { | ||
const filter = this._customDataBinFilterSet.get(uniqueKey); | ||
return filter === undefined | ||
? false | ||
: (filter!.showPreview as boolean); | ||
} else { | ||
const filter = this.getDataBinFilterSet(uniqueKey).get(uniqueKey)!; | ||
return filter?.showPreview === undefined | ||
? false | ||
: (filter!.showPreview as boolean); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this could be simplified. one way could be to use ternary operator (n ? x : y) to get the filter and something like return filter?.showPreview || false
for the return value
@@ -74,7 +84,37 @@ export default class BarChart extends React.Component<IBarChartProps, {}> | |||
|
|||
constructor(props: IBarChartProps) { | |||
super(props); | |||
this.state = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we mainly use mobx for state management so instead of this.state and setState we use decorators like @observable
and @action
. can we look into this and see if we can do that?
if (!domain || !domain.x) { | ||
console.error('Brush domain is not defined.'); | ||
return; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lets remove this and just check if domain
and domain.x
are defined and if so do the actions below.
7b990ad
to
d48bfc7
Compare
Add Histogram Zoom Preview: